fix: adjust SmoothieChart minValue and scaling for improved chart acc…#2382
Conversation
…uracy in DashStats page
WalkthroughUpdates chart configuration in DashStats.page: sets CPU chart minValue to 0, removes yRangeFunction, and applies minValueScale/maxValueScale of 1.02 to CPU and network charts, introducing symmetrical 2% margins and preventing negative CPU values. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/DashStats.page (2)
1466-1468: minValueScale/maxValueScale are no-ops with fixed min/max; pick one approachWith minValue: 0 and maxValue: 100 set, Smoothie’s min/max scaling factors won’t change the range. If you want visible headroom, drop the fixed max; if you want a fixed 0–100 scale, remove the scales to avoid confusion.
Option A (fixed range 0–100; remove scales):
- minValueScale: 1.02, - maxValueScale: 1.02,Option B (dynamic headroom; drop fixed max so maxValueScale applies):
- maxValue: 100, + // Let Smoothie compute dynamic headroom near 100% + // maxValue: 100,Please confirm the intended behavior (fixed 0–100 vs. padded headroom). If choosing Option B and you want labels capped at 100, we can also clamp yMaxFormatter to 100.
1493-1495: Net chart: minValueScale is ineffective with minValue=0; keep only maxValueScaleBecause minValue is fixed at 0, minValueScale won’t create a lower margin. Keeping maxValueScale=1.02 provides the desired 2% headroom at the top.
Apply:
- minValueScale: 1.02, maxValueScale: 1.02,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/DashStats.page(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T21:22:09.359Z
Learnt from: elibosley
PR: unraid/webgui#2378
File: emhttp/plugins/dynamix/DashStats.page:1440-1446
Timestamp: 2025-09-17T21:22:09.359Z
Learning: For SmoothieCharts millisPerPixel calculation, use element.offsetWidth (CSS pixels) rather than canvas.width or devicePixelRatio adjustments. The library appears to handle DPI scaling internally, and using canvas pixel width gives incorrect timing on HiDPI displays.
Applied to files:
emhttp/plugins/dynamix/DashStats.page
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
emhttp/plugins/dynamix/DashStats.page (2)
1450-1452: Setting CPU chart minValue=0 is correct to prevent negative CPU valuesKeeps the axis grounded at zero and matches the intent to avoid sub‑zero CPU.
1448-1474: Smoothie options validated — minValueScale/maxValueScale supportedplugins/dynamix/javascript/smoothie.js documents and implements minValueScale/maxValueScale (comments at 305–307), sets defaults (421–422) and uses them in calculations (810, 817).
🧹 PR Test Plugin Cleaned UpThe test plugin and associated files for this PR have been removed from the preview environment. 🤖 This comment is automatically generated when a PR is closed. |
…uracy in DashStats page
Summary by CodeRabbit
Bug Fixes
Style